-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Slice/Array intrinsics: __slice
and __elem_at
#6282
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xunilrj
force-pushed
the
xunilrj/slice-intrinsics
branch
from
July 22, 2024 11:23
a7b698e
to
ca2daa3
Compare
Benchmark for d3d6dc2Click to view benchmark
|
Benchmark for 79c9a60Click to view benchmark
|
Benchmark for 421d20cClick to view benchmark
|
Benchmark for f6b311dClick to view benchmark
|
xunilrj
force-pushed
the
xunilrj/slice-intrinsics
branch
from
July 25, 2024 12:52
00d39d8
to
47629a7
Compare
Benchmark for 43ea4e9Click to view benchmark
|
Benchmark for 53dd4f7Click to view benchmark
|
Benchmark for b8585c8Click to view benchmark
|
xunilrj
changed the title
Slice intrinsics:
Slice/Array intrinsics: Jul 27, 2024
__slice
and __slice_elem
__slice
and __elem_at
Benchmark for 18a75c1Click to view benchmark
|
IGI-111
reviewed
Jul 30, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but these new intrinsics should be documented.
xunilrj
force-pushed
the
xunilrj/slice-intrinsics
branch
from
July 31, 2024 13:51
4cf6137
to
514ddf8
Compare
Benchmark for f6140fdClick to view benchmark
|
Benchmark for 2cfce5fClick to view benchmark
|
IGI-111
previously approved these changes
Aug 2, 2024
xunilrj
force-pushed
the
xunilrj/slice-intrinsics
branch
from
August 4, 2024 15:39
77cd595
to
11c96d3
Compare
Benchmark for 74559ccClick to view benchmark
|
IGI-111
approved these changes
Aug 5, 2024
tritao
approved these changes
Aug 5, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some minor suggestions.
Benchmark for 00c516fClick to view benchmark
|
kayagokalp
approved these changes
Aug 7, 2024
Benchmark for e26acdeClick to view benchmark
|
esdrubal
pushed a commit
that referenced
this pull request
Aug 13, 2024
## Description This PR is part of #5110 and introduces two new intrinsic: `__slice` and `__elem_at`. `__slice` allows the creation of slices by slicing arrays or other slices. Whilst `__elem_at` returns a reference to an item inside the slice or the array. ## Out of bounds checks These intrinsic will not generate any runtime checks, these must be done manually, when and where appropriate; but they do a complete static analysis of all indices, to avoid runtime buffer overflows, when possible. That means that at runtime, it is possible to do a buffer overflow when reading/writing, which is an "undefined behaviour" as to what will happen. ## Empty Array This PR also solves a problem with empty arrays. Before empty arrays such as `let a = []` were being type-checked as `[Never; 0]`, which means that any code after them was being marked as dead. Now we correctly type check them as `[Unknown; 0]` and return a more friendly error. ``` 4 | 5 | // Empty array 6 | let a = []; | ^^ Type must be known at this point 7 | } | ____ ``` ## Check of constants inside fns This PR also solves a problem with not checking `const` expressions inside `fns`. We, for example, do not allow slices in constants, but we were only checking globals. Now we check constants also inside functions, methods etc... ## Small improvements for our e2e We can now `dbg` inside our e2e harness and get results like the ones below. One needs to include the lib `test/src/e2e_vm_tests/utils` and cal `something.dbg()` or `something.dbgln()`. There is no magic, and structs/enums will need to manually implement the `Dbg` trait. This is only to facilitate the debugging of our e2e tests. ![image](https://github.com/user-attachments/assets/2f25c50e-b7b3-4199-8bf4-699473919e6c) ## Checklist - [ ] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is part of #5110 and introduces two new intrinsic:
__slice
and__elem_at
.__slice
allows the creation of slices by slicing arrays or other slices. Whilst__elem_at
returns a reference to an item inside the slice or the array.Out of bounds checks
These intrinsic will not generate any runtime checks, these must be done manually, when and where appropriate; but they do a complete static analysis of all indices, to avoid runtime buffer overflows, when possible.
That means that at runtime, it is possible to do a buffer overflow when reading/writing, which is an "undefined behaviour" as to what will happen.
Empty Array
This PR also solves a problem with empty arrays. Before empty arrays such as
let a = []
were being type-checked as[Never; 0]
, which means that any code after them was being marked as dead.Now we correctly type check them as
[Unknown; 0]
and return a more friendly error.Check of constants inside fns
This PR also solves a problem with not checking
const
expressions insidefns
. We, for example, do not allow slices in constants, but we were only checking globals. Now we check constants also inside functions, methods etc...Small improvements for our e2e
We can now
dbg
inside our e2e harness and get results like the ones below. One needs to include the libtest/src/e2e_vm_tests/utils
and calsomething.dbg()
orsomething.dbgln()
. There is no magic, and structs/enums will need to manually implement theDbg
trait.This is only to facilitate the debugging of our e2e tests.
Checklist
Breaking*
orNew Feature
labels where relevant.